Hide double pin card when use pinyin is false & additional tests #3831
Conversation
VictoriousRaptor
added
Dev branch only
VictoriousRaptor
force-pushed
the
tweak-double-pin-pane
branch
from
1e06799 to
fd4efe0
Compare
|
Be a legend 🏆 by adding a before and after screenshot of the changes you made, especially if they are around UI/UX. |
📝 WalkthroughWalkthroughA visibility binding was added to the "ShouldUseDoublePinyin" card in the settings pane, making it visible only when the "ShouldUsePinyin" property is true. This uses a custom markup extension for the visibility logic. No other controls or logic were changed. Additionally, the spelling exception list for the GitHub action was updated with nine new proper nouns. A new test class was added to benchmark and verify the correctness of Chinese character detection methods. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant SettingsPaneGeneral.xaml
participant ViewModel
User->>SettingsPaneGeneral.xaml: Opens General Settings
SettingsPaneGeneral.xaml->>ViewModel: Bind ShouldUsePinyin
alt ShouldUsePinyin is true
SettingsPaneGeneral.xaml->>SettingsPaneGeneral.xaml: Show "ShouldUseDoublePinyin" card
else ShouldUsePinyin is false
SettingsPaneGeneral.xaml->>SettingsPaneGeneral.xaml: Hide "ShouldUseDoublePinyin" card
end
Possibly related PRs
Suggested reviewers
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
This comment has been minimized.
This comment has been minimized.
@check-spelling-bot Report🔴 Please reviewSee the 📂 files view, the 📜action log, or 📝 job summary for details.
See ❌ Event descriptions for more information. If the flagged items are 🤯 false positivesIf items relate to a ...
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
Flow.Launcher.Test/ChineseDetectionPerformanceTest.cs (1)
66-72: Consider using modern NUnit assertion syntax.The tests use
ClassicAssertfrom the legacy namespace. Consider migrating to the modern constraint-based syntax for better readability and consistency with current NUnit best practices.For example, replace:
-ClassicAssert.IsFalse(ContainsChinese("Hello World"), "Pure English should return false"); +Assert.That(ContainsChinese("Hello World"), Is.False, "Pure English should return false");-ClassicAssert.AreEqual(wordsHelperResult, containsChineseResult, - $"Results differ for string: '{testString}'. WordsHelper: {wordsHelperResult}, ContainsChinese: {containsChineseResult}"); +Assert.That(containsChineseResult, Is.EqualTo(wordsHelperResult), + $"Results differ for string: '{testString}'. WordsHelper: {wordsHelperResult}, ContainsChinese: {containsChineseResult}");Also applies to: 78-84, 95-96
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
Flow.Launcher.Infrastructure/UserSettings/Settings.cs(1 hunks)Flow.Launcher.Test/ChineseDetectionPerformanceTest.cs(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- Flow.Launcher.Infrastructure/UserSettings/Settings.cs
🧰 Additional context used
🪛 GitHub Check: Check Spelling
Flow.Launcher.Test/ChineseDetectionPerformanceTest.cs
[warning] 141-141:
Finalizers is not a recognized word. (unrecognized-spelling)
[warning] 6-6:
NUnit is not a recognized word. (unrecognized-spelling)
🪛 GitHub Actions: Check Spelling
Flow.Launcher.Test/ChineseDetectionPerformanceTest.cs
[warning] 6-6: Spell check warning: NUnit is not a recognized word. (unrecognized-spelling)
[warning] 141-141: Spell check warning: Finalizers is not a recognized word. (unrecognized-spelling)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: gitStream workflow automation
- GitHub Check: gitStream.cm
- GitHub Check: gitStream.cm
- GitHub Check: build
🔇 Additional comments (1)
Flow.Launcher.Test/ChineseDetectionPerformanceTest.cs (1)
12-61: LGTM! Well-structured test class with comprehensive test data.The test data covers all important scenarios including pure English, pure Chinese, mixed content, edge cases, and performance testing strings.
jjw24
changed the title
|
🥷 Code experts: Jack251970, onesounds VictoriousRaptor, Jack251970 have most 👩💻 activity in the files. See details
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame: ✨ Comment |
3 participants
Hide double pin card when use pinyin is false.
Follows on from #2427